fix(nft): re-evaluate previous owner's eligibility on NFT ownership change#243
Open
rdmcd wants to merge 1 commit intoOpenBuilders:mainfrom
Open
fix(nft): re-evaluate previous owner's eligibility on NFT ownership change#243rdmcd wants to merge 1 commit intoOpenBuilders:mainfrom
rdmcd wants to merge 1 commit intoOpenBuilders:mainfrom
Conversation
…hange When fetch_wallet_details runs for the new owner of an NFT, the previous owner is never queued into UPDATED_WALLETS_SET_NAME, so check-chat-members does not re-evaluate them. They keep sitting in any chat whose rules required this NFT, even though the DB already reflects that they no longer own it. NftItemService.bulk_create_or_update now returns the set of addresses that lost ownership in the batch alongside the persisted items. The caller (indexer_blockchain.tasks.fetch_wallet_details) forwards those addresses into UPDATED_WALLETS_SET_NAME so check-chat-members picks them up via the existing path. The service stays data-access only; the orchestration owns the side-effects.
danoctua
approved these changes
May 6, 2026
Collaborator
danoctua
left a comment
danoctua
left a comment
There was a problem hiding this comment.
OK as a hot fix to mitigate side effects, but still won't work if tracked user transfers NFT to non-tracked, and event for tracked is not sent – we should work on improving the stability of delivered events for the long term instead.
rdmcd
force-pushed
the
fix/nft-update-evicts-previous-owner-main
branch
from
67a0d5c to
8e19b72
Compare
danoctua
approved these changes
May 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
When
fetch_wallet_detailsruns for the new owner of an NFT (because thetransaction tracker observed activity on that address),
NftItemService.bulk_create_or_updateoverwritesnft.owner_addressfromA to B. After completion, only the new owner's address (B) is added to
UPDATED_WALLETS_SET_NAME, socheck-chat-membersonly re-evaluates B —who is correctly eligible.
The previous owner A is never put into
UPDATED_WALLETS_SET_NAME, socheck-chat-membersdoes not re-evaluate them. They keep sitting in anychat whose rules required this NFT, even though the DB already reflects
that they no longer own it.
Fix
NftItemService.bulk_create_or_updatenow returns a second value — theset of addresses that lost ownership in this batch — alongside the
persisted items. The caller
(
indexer_blockchain.tasks.fetch_wallet_details) forwards those addressesinto
UPDATED_WALLETS_SET_NAMEsocheck-chat-membersre-evaluates them onits next tick and kicks them via the existing path if the rule no longer
holds.
The Redis side-effect is intentionally kept out of the service layer — the
service stays data-access only, and the orchestration (Celery task) owns
the side-effects.
Tests
tests/unit/core/services/test_nft_item_service.py:_createpath) → empty set